Skip to content

Conversation

gbebgdee
Copy link

@gbebgdee gbebgdee commented Jul 9, 2021

No description provided.

@gbebgdee gbebgdee marked this pull request as draft July 9, 2021 03:55
@gbebgdee gbebgdee marked this pull request as ready for review July 9, 2021 03:55
@balthisar
Copy link
Member

I like it (see #519), but can you clean up the commit a little bit? We shouldn't leave the console application with the callback code activated, for example, as well as the other commented out code.

More importantly, are you certain that the generic approach works for EVERY existing string? A quick scan doesn't look like there would be any issues. One concern, though, is if you take a look at e.g., TC_TXT_HELP_3, by swallowing the newlines, you're also swallowing the vertical spacing. Maybe it's not a concern.

I'm looking at this quickly, but if a string ends with two newlines, are you going to overrun the string that you're copying?

I wonder if it's not time to finally just remove the newlines from the source, and perform wrapping in the console application. This has been done and rejected before.

Let me know your thoughts.

Whatever the case, I may defer this a couple of weeks in order to issue a stable 5.8 release with strings as they are, and make this the first commit on 5.9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants